Skip to content

TELCORE-102: Add Unified Plan BUNDLE RTP receive demux - #620

Open
baloeng wants to merge 2 commits into
telnyx/telephony/deploy-developmentfrom
linear-telcore-102-unified-plan-bundle
Open

TELCORE-102: Add Unified Plan BUNDLE RTP receive demux#620
baloeng wants to merge 2 commits into
telnyx/telephony/deploy-developmentfrom
linear-telcore-102-unified-plan-bundle

Conversation

@baloeng

@baloeng baloeng commented Jul 6, 2026

Copy link
Copy Markdown

Summary

  • Replaces TEL-6738: Add BUNDLE RTP receive demux #575 with the same Unified Plan / BUNDLE FreeSWITCH changes squashed into a single commit on linear-telcore-102-unified-plan-bundle.
  • Add FreeSWITCH RFC 9143 BUNDLE / Unified Plan groundwork for audio+video over one RTP transport, behind the BUNDLE policy path rather than default-on behavior.
  • Add the BUNDLE data model and helpers for offered/accepted MIDs, m-line lookup, RTP demux, and bundle-state handling.
  • Add RTP MID extension support: runtime-gated outbound MID writes, inbound MID parsing, extension-state reset/strip handling, and tests for MID/extension behavior.
  • Protect shared BUNDLE RTP ownership so the audio RTP session owns the shared socket while bundled video borrows that transport safely.
  • Route bundled RTP receive traffic by MID first, learned SSRC second, and unique payload type fallback; queue video packets to the video read path while keeping audio on the shared audio path.
  • Preserve RTP packet layout when queueing BUNDLE frames, including CSRC/header-extension packets, by cloning into full packet storage instead of unsafe frame-buffer dup/free paths.
  • Fix BUNDLE media correctness issues found during testing: video SSRC/MID/timestamp handling, isolated video write state, audio payload-type contamination on the shared RTP session, and stale payload-map reset around queued video reads.
  • Improve startup/recovery behavior for WebRTC video: buffer/drop pre-DTLS video safely, request a fresh source keyframe after readiness, gate startup VP8 video until a real keyframe, and guard keyframe refresh during teardown.
  • Fix BUNDLE RTCP feedback handling: inbound FIR/PLI receive and partner propagation, RTCP feedback during warmup, and lower bridge PLI relay throttling for faster keyframe recovery.
  • Add and update unit coverage for BUNDLE negotiation/data-model behavior, RTP MID handling, pcap/RTP parsing, and Unified Plan/BUNDLE cases.
  • Remove review/debug leftovers from the branch, including TEL-6738-specific probe names, the temporary design doc, noisy BUNDLE DEBUG logs, and now-dead debug counters.

No mod_telnyx_rtc files are changed in this PR; signaling-side enablement is handled separately.

@baloeng baloeng mentioned this pull request Jul 6, 2026
Closed
@dev-ryanc

dev-ryanc commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Code Review — PR #620 (BUNDLE / Unified Plan)

Verdict: Reviewed 💬 (3 warnings, 0 critical)

⚠️ Warning — src/switch_rtp.c:11999 — Predictable SSRC
(intptr_t)rtp_session + epoch_time is guessable from outside the process. On BUNDLE legs where demux trusts SSRC, this enables RTP injection. Use switch_crypto_secure_rng() or apr_generate_random_bytes() for entropy.

⚠️ Warning — src/switch_rtp.c:12843 — atoi without validation
atoi(tok) returns 0 on non-numeric input and silently overflows on large values. Malformed ICE candidates would pass component_id=0 / port=0 to the ICE engine — a crafted SDP could crash or confuse the B2BUA. Same issue at line 12850 (port = atoi(tok)). Use strtol with endptr validation.

⚠️ Warning — src/switch_utils.c:1392 — switch_is_lan_addr behavior change
strncmp(ip, "1.", 2) and "2." exclusions removed. 1.0.0.0/8 (APNIC/Cloudflare) and 2.0.0.0/8 (RIPE) are public internet. This changes NAT detection and ICE candidate filtering. If intentional, add a comment explaining the rationale.

Inline comments not available — GitHub API truncates the diff for files >100KB. Reviewed from local bare repo full diff.
Reviewed by Clawbot

Emmanuel Balogun
Merge remote-tracking branch 'origin/telnyx/telephony/deploy-developm…
3c5a09c 
…ent' into linear-telcore-102-unified-plan-bundle
dev-ryanc
dev-ryanc requested changes Aug 7, 2026
View reviewed changes

@dev-ryanc dev-ryanc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 finding: memory leak in write-frame resample path.

[MEDIUM] switch_core_media.c — resample_tmp memory leak when both write and read resampler overflow in one frame

// switch_core_media.c:21519 — free prior allocation before overwrite:
+  if (resample_tmp) { free(resample_tmp); resample_tmp = NULL; }
   switch_malloc(resample_tmp, out_bytes);

Both the write resampler (L21132) and read resampler (L21519) overflow paths share the same resample_tmp variable. When both overflow in a single switch_core_session_write_frame call, the read resampler's switch_malloc overwrites the write resampler's allocation without freeing it — a per-frame leak of out_bytes (typically a few KB).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dev-ryanc dev-ryanc dev-ryanc requested changes
@damirn damirn Awaiting requested review from damirn
@minhtuan1407-telnyx minhtuan1407-telnyx Awaiting requested review from minhtuan1407-telnyx
@tajamulTelnyx tajamulTelnyx Awaiting requested review from tajamulTelnyx

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@baloeng @dev-ryanc